Skip to content

docs: update readme to use defineConfig#42

Merged
43081j merged 1 commit intomainfrom
docs-define
Apr 9, 2025
Merged

docs: update readme to use defineConfig#42
43081j merged 1 commit intomainfrom
docs-define

Conversation

@43081j
Copy link
Contributor

@43081j 43081j commented Apr 3, 2025

No description provided.

@43081j
Copy link
Contributor Author

43081j commented Apr 4, 2025

@tbroyer does this look right to you now?

Copy link
Contributor

@tbroyer tbroyer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would have to try that it actually works, but looks good to me

@tbroyer
Copy link
Contributor

tbroyer commented Apr 4, 2025

Trying it locally (using npm add -D eslint-plugin-depend@file:../../e18e/eslint-plugin-depend, I have no idea how npm link actually works and whether it's/there's a better solution, it just scares me as I don't want any system-wide side effect), this still fails with the same error as in #43.

Changing the project to emit ESM (I added "type": "module" to the package.json and ran npm run build:js; eslint.config.js would have to be migrated to ESM or renamed to .cjs) makes it pass though. Given that Node 18 reaches EOL at the end of the month, maybe that's an acceptable change?

@43081j
Copy link
Contributor Author

43081j commented Apr 4, 2025

yep i actually opened #45 today and had meant to do it a few weeks ago but totally forgot

we can land that first in a new major and merge this afterwards maybe

@43081j 43081j merged commit 6985c03 into main Apr 9, 2025
3 checks passed
@43081j 43081j deleted the docs-define branch April 9, 2025 09:19
@binarykitchen
Copy link

Well, if you do that, there will be this TS error:

Type 'string' is not assignable to type 'InfiniteDepthConfigWithExtends'.

image

To reproduce, add on top of file // @ts-check

@43081j
Copy link
Contributor Author

43081j commented Jul 4, 2025

in ESLint 9, it works fine (it was introduced in 9.x)

in earlier versions of 9, and 8, you have to use a reference (depend.configs['flat/recommended'])

@binarykitchen
Copy link

Hmmm, for me, I've figured this workaround:

import * as depend from "eslint-plugin-depend";

... and this:

  {
    name: "depend",
    ...depend.configs["flat/recommended"],
    rules: {
      "depend/ban-dependencies": [
        "error",
        {
          Allowed: ["lodash"],
        },
      ],
    },
  },

@43081j
Copy link
Contributor Author

43081j commented Jul 4, 2025

you can extends the reference:

extends: [
  depend.configs['flat/recommended']
]

@binarykitchen
Copy link

binarykitchen commented Jul 4, 2025

Well, why is this better? And this causes another problem. For example, if I use

  {
    name: "depend",
    extends: [depend.configs["flat/recommended"]],
    rules: {
      "depend/ban-dependencies": [
        "error",
        {
          allowed: ["lodash"],
        },
      ],
    },
  },

then the ESLint inspector displays it twice, one time too much:

image

This happens when you use extend, but not when you use the spread operator.

@tbroyer
Copy link
Contributor

tbroyer commented Jul 4, 2025

IMO the question is not whether this is better or not (for some definition of "better" that'll probably be subjective), but whether this follows ESLint's own choices (whether you like it not).

ESLint's documentation uses defineConfig() and extends, so that's what an ESLint plugin should also use in its docs IMO.

And ESLint v8 was EOL'd back in october last year: https://eslint.org/blog/2024/09/eslint-v8-eol-version-support/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants